Skip to content

fix(BA-6085): seed reads_vfolder_config_files=true for custom variant#11661

Open
seedspirit wants to merge 4 commits into
mainfrom
fix/BA-6085
Open

fix(BA-6085): seed reads_vfolder_config_files=true for custom variant#11661
seedspirit wants to merge 4 commits into
mainfrom
fix/BA-6085

Conversation

@seedspirit
Copy link
Copy Markdown
Contributor

Summary

  • The original runtime_variants seed migration (9229f72fa447) inserted rows with only name / description, so reads_vfolder_config_files took its server default of false for every variant.
  • populate_fixture uses INSERT … ON CONFLICT DO NOTHING, so the install fixture's "reads_vfolder_config_files": true for custom is silently dropped on databases that already have the seed row.
  • Add a one-shot migration that UPDATE runtime_variants SET reads_vfolder_config_files = true WHERE name = 'custom' so existing databases get the intended flag and the vfolder model-definition.yaml scan kicks in at revision-creation time.

Test plan

  • alembic upgrade head from ba42cb865efe: custom becomes true, other 6 variants untouched
  • alembic downgrade -1: custom reverts to false, version returns to ba42cb865efe
  • Re-running upgrade head on a database where custom is already true is a no-op (UPDATE is idempotent)
  • After upgrade, v2 add_revision with custom variant and a model vfolder persists the resolved model_definition_path (model-definition.yaml) instead of an empty string

Resolves BA-6085

The original runtime_variants seed migration inserted rows with only
name/description, and populate_fixture's ON CONFLICT DO NOTHING never
overwrites the existing rows, so the install fixture's
reads_vfolder_config_files=true for custom was silently dropped. Backfill
the value via a new migration so custom actually scans model-definition
files from the vfolder at revision-creation time.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions github-actions Bot added size:M 30~100 LoC comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated labels May 18, 2026
seedspirit and others added 2 commits May 18, 2026 17:39
@seedspirit seedspirit marked this pull request as ready for review May 18, 2026 09:05
@seedspirit seedspirit requested a review from a team as a code owner May 18, 2026 09:05
Copilot AI review requested due to automatic review settings May 18, 2026 09:05
@seedspirit seedspirit added this to the 26.4 milestone May 18, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a one-shot Alembic migration to backfill reads_vfolder_config_files=true for the seeded custom runtime variant, so custom model-serving revisions can read model-definition.yaml from the model vfolder as intended.

Changes:

  • Adds a database migration that updates the custom runtime variant flag on upgrade and resets it on downgrade.
  • Adds a fix news fragment describing the model-definition loading fix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ai/backend/manager/models/alembic/versions/0b10b2c6a972_seed_reads_vfolder_for_custom_variant.py Adds the backfill migration for custom.reads_vfolder_config_files.
changes/11661.fix.md Adds the release-note fragment for the custom runtime variant fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +39 to +43
op.execute(
sa.text(
"UPDATE runtime_variants SET reads_vfolder_config_files = false WHERE name = 'custom'"
)
)
The prior revision 7ea9f3c1b2d5 already sets custom.reads_vfolder_config_files
to TRUE during column creation, so unconditionally writing FALSE on downgrade
clobbered that valid prior state. Drop the UPDATE so downgrade leaves the row
untouched.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants